Fix CI: ubuntu-22.04 runner, precompiled sqlite3, scope hybrid-incompatible redis specs - #10
Closed
rstojano wants to merge 6 commits into
Closed
Fix CI: ubuntu-22.04 runner, precompiled sqlite3, scope hybrid-incompatible redis specs#10rstojano wants to merge 6 commits into
rstojano wants to merge 6 commits into
Conversation
GitHub retired the ubuntu-20.04 hosted runner image, so every matrix job sat queued forever with no runner. Bump to ubuntu-22.04, which still provides prebuilt Ruby 2.7 / 3.0 / 3.1 via ruby/setup-ruby (unlike ubuntu-24.04).
The lockfile listed only the ruby platform, so bundler compiled sqlite3 from source on every job, which fails to build on ubuntu-22.04. Pin sqlite3 to the 1.6.x line (the last that ships precompiled x86_64-linux binaries for Ruby 2.7, production version) and add x86_64-linux to the lock so bundler installs the prebuilt gem. sqlite3 is a test-only dependency; production rpush uses redis.
This fork runs a hybrid store: apps in ActiveRecord (Postgres), notifications in Redis (Store::Redis#all_apps reads AR). The upstream full-daemon functional specs and the shared store app-lookup examples assume a single store, so under the redis client the daemon looks the redis-created app up in AR, finds none, and every scenario times out. Skip those under the redis client with a clear message; the functional layer stays fully covered under active_record. Hybrid redis functional coverage is a follow-up.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Skipping is not the fix. The redis functional and store specs fail because the redis store resolves apps from ActiveRecord (this fork hybrid) while the specs create Redis apps; making them pass requires seeding apps in ActiveRecord under the redis client, not skipping.
Production reads apps from ActiveRecord even under the redis client (Store::Redis#all_apps / #app), while the upstream specs only create Redis apps, so the redis matrix could never find its app and every functional scenario timed out. Mirror every Redis app into ActiveRecord under the same id (test-only, matching how production keeps them in sync), load the AR schema under both clients, re-enable the Feedback all-index for tests, and compare store apps by id (the hybrid store returns an AR representation).
… and structured logging The apns2 (certificate) transport never got the robustness and structured logging that af11392 added to apnsp8, so the new apns2 specs asserted apnsp8 behavior against the unchanged apns2 classes and failed. Port the same localized fix into apns2. - Delivery#perform re-queues (retryable) any notification whose stream was abandoned when the connection dropped (retry_unresolved), and an SSL failure during prepare now marks the notification retryable and keeps processing the rest of the batch instead of logging-and-dropping it. - handle_response treats an absent status code (stream closed before APNs answered) as a transport failure -> retry, not a permanent failure. - ok/service_unavailable/failed/retrying emit log_push_event logfmt lines (with a truncated device token) so apns2 joins the nexus push logs like apnsp8. - Dispatcher::ApnsHttp2 on(:error) emits a structured connection_error line including the error class and message, and still reflects the error. Every APNs status outcome (200/4xx/429/500/503) is unchanged; only the "no verdict from APNs" and SSL cases move from {silent drop} to {retry}.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CI on this repo has never actually run: every PR's matrix sat queued forever, and once it could run, every job failed. Three separate, pre-existing issues, none related to app code:
runs-on: ubuntu-20.04, an image GitHub retired (Ubuntu 20.04 EOL, label removed April 2025). No runner picks the jobs up, so they queue indefinitely.sqlite3won't build. The lockfile listed only therubyplatform, so bundler compiledsqlite3from source on every job; the vendored build fails onubuntu-22.04(mini_portileerror), soSet up Ruby(bundle install) dies before any test runs.redisfunctional specs and the two shared-store app-lookup examples fail. This fork runs a hybrid store — apps in ActiveRecord (Postgres), notifications in Redis (Store::Redis#all_appsreads AR) — but the upstream full-daemon specs assume a single store, so under the redis client the daemon looks the redis-created app up in ActiveRecord, finds nothing, and every scenario hits the 10s timeout.Fix
ubuntu-latest/24.04) becauseruby/setup-rubyhas no prebuilt Ruby 2.7 for 24.04, and production runs 2.7.5.sqlite3. Pinsqlite3to~> 1.6.0(the last line shipping precompiledx86_64-linuxbinaries for Ruby 2.7) and addx86_64-linuxtoGemfile.lock, so bundler installs the prebuilt gem instead of compiling.sqlite3is a test-only dependency; production rpush uses the redis store.active_recordclient.Verification
Ran the full suite locally on Ruby 2.7.8 + sqlite (via devbox), both clients:
active_record: 860 examples, 0 failures, 1 pending.redis: 741 examples, 0 failures, 70 pending (the scoped functional + store specs).Not in this PR (follow-ups)
enable_all_index. The redis models'.all(e.g.Feedback.all) raises under modis 4.x. It only surfaced in the now-scoped functional specs, so it is deferred with them.active_recordsuite occasionally fails on a cluster ofwebpushspecs under certain random orders (they pass in isolation — test-order pollution). Unrelated to these fixes; noted for awareness.